Press2 1233 new modern layout component #338
Conversation
ajayadav09
commented
Oct 18, 2023
ajayadav09
commented
|
fixed linting issues |
ajayadav09
added
Code Review
| progressbar={ | ||
| <ProgressBarSiteGen current={ 20 } total={ 100 } /> | ||
| } |
There was a problem hiding this comment.
I think we can do the percent calc inside the component itself.
There was a problem hiding this comment.
yes this is just for representation. we will need to pass the necessary data (current value and total value) as props to the component. Then, we can perform the calculation within the component itself
There was a problem hiding this comment.
I think the approach of adding SiteGen won't be long term and we might need a folder structure like:
|-- src
|-- OnboardingSPA
| |-- components
| |-- steps etc...
| |-- index.js
|-- SiteGenSPA
| |-- components
| |-- steps etc...
| |-- index.js
|-- Utils
| |-- Common utils like Live Preview
|-- index.js
Can we do something like this to keep the code separate?
Maybe we can discuss this?
src/OnboardingSPA/components/SiteGenInterfaceSkeleton/style.scss
Outdated
Show resolved
Hide resolved
…oarding into PRESS2-1233-new-modern-layout-component
There was a problem hiding this comment.
Few Things:
- Let's get rid of
SiteGenprefix/suffix in folders? - Let's not use
--nfd-onboarding-*and maybe--nfd-sitegen-*or--nfd-sg-*
I feel if the prefix are not attached to css classes then a change in sitegen might break onboarding and then go undetected.
…bs/wp-module-onboarding into PRESS2-1233-new-modern-layout-component # Conflicts: # src/OnboardingSPA/data/flows/default.js
|
This has been addressed in #362. |
